| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-04-27 | |||
| 13:54:11 | zigo | I should do the former, to have the file owned by root:nova / 640 though ... | |
| 13:56:45 | gmann | zigo: let me check if that tool adding the default rule or not. | |
| 13:59:25 | zigo | gmann: I've sent the generated policy.json file to our swift cluster if you want to look at it: https://www.swisstransfer.com/d/b80904d3-1f15-4f1f-98f0-7e1db308bb53 | |
| 14:13:59 | openstackgerrit | Kevin Zhao proposed openstack/nova master: [WIP] CI: add tempest-integrated-compute-aarch64 job https://review.opendev.org/714439 | |
| 14:14:40 | gmann | zigo: got it. that tool does not add the deprecated rules in sample file. | |
| 14:15:10 | zigo | gmann: And that's the issue ?!? | |
| 14:15:22 | gmann | zigo: expectation is you keep only override rule in the policy file and other rule let it rely on defaults | |
| 14:15:49 | zigo | Right. | |
| 14:15:51 | gmann | zigo: not issue i think. because you are providing the file with rule override with new defaults | |
| 14:16:55 | gmann | if you generate the file with that tool you get all the rule commented and you are supposed to un-comment the one you want to override. | |
| 14:17:34 | gmann | here what happen, nova get the rule in file and skip the default value with consideration that rule in file is what operator want | |
| 14:18:12 | gmann | if you remove the rules from file which you want to reply on defaults then your old token will keep working. | |
| 14:23:31 | gmann | zigo: also if rule is present in file then oslo skip deprecated rule to add. and I hope you generated file before nova start which initialize the policy | |
| 14:29:42 | zigo | gmann: If I remove the policy.json, then I get an error 500: | |
| 14:29:42 | zigo | [pid: 1708|app: 0|req: 10/40] 192.168.101.2 () {32 vars in 628 bytes} [Mon Apr 27 14:29:08 2020] GET /v2.1/flavors/detail => generated 128 bytes in 91 msecs (HTTP/1.1 500) 3 headers in 215 bytes | |
| 14:29:47 | zigo | Nothing more in the logs ... | |
| 14:32:48 | zigo | gmann: The file needs to exist, though if it's empty, it looks like working ! :) | |
| 14:32:56 | zigo | gmann: Should I keep an empty file then?!? | |
| 14:33:22 | zigo | IMO this is still a bug, because operators need to see what's currently in the policy, and can't guess the defaults. | |
| 14:33:46 | zigo | I do want to provide such a policy file if possible. | |
| 14:33:59 | zigo | gmann: An empty policy.json is safe, right? | |
| 14:34:00 | dansmith | if operators currently have to do anything to their policy file during an upgrade, then we have a real problem | |
| 14:34:28 | dansmith | zigo: AFAIK, the policy file should be empty to take all the defaults, but I'm surprised it has to be present-but-empty.. not sure if that is new or not | |
| 14:34:52 | zigo | dansmith: I expect operators to use /etc/nova/policy.d, and I thought about explicitly shipping such a folder in the Nova Debian package. | |
| 14:35:17 | zigo | As much as I can tell, this is a new bug ! :P | |
| 14:35:25 | zigo | (would have to check Train though...) | |
| 14:35:27 | dansmith | zigo: and thus have no files in there nor an empty base file right? | |
| 14:36:29 | zigo | dansmith: What would happen if a rule is defined in both /etc/nova/policy.json and /etc/nova/policy.d/foo-operator.json ? | |
| 14:36:41 | zigo | Will the policy.d have priority? | |
| 14:36:49 | dansmith | no idea.. I didn't know we had a policy.d, tbh | |
| 14:36:54 | zigo | Beause that'd be the most convenient way for everyone. | |
| 14:37:13 | zigo | We do need a way to tell operators what they can and cannot write in their config. | |
| 14:37:20 | dansmith | but I would expect a distro to install an empty policy.d directory, and not have to write an empty base policy file to avoid a 500 | |
| 14:38:07 | zigo | dansmith: What I'm going to do is to write an empty policy.json (to avoid what I consider a bug), ship the generated policy.json in /usr/share/nova-common as an example, and create the policy.d folder. | |
| 14:38:30 | zigo | I still think it's wrong that I can't use the generated policy.json though... | |
| 14:38:42 | dansmith | zigo: ack, but if the behavior is changed, we need a bug filed | |
| 14:39:14 | zigo | It really is changed. I use to ship the /etc/nova/policy.json on all of my Nova packages, and so far, it wasn't a problem. | |
| 14:39:15 | dansmith | zigo: well, we're trying to get people to have overrides and not hard-coded everything, but I understand.. what prevents you from using the generated file? deprecation warnings? | |
| 14:39:44 | zigo | It simply does *not* work. | |
| 14:39:56 | gmann | dansmith: zigo file generated from tool is kind of override rule. default only work if rule not in file | |
| 14:40:33 | dansmith | zigo: but why is the generated file not working? | |
| 14:41:03 | gmann | zigo: did that worked for any deprecated rule for you, if you rule in file the if any rule deprecated in fast had same issue itthunk | |
| 14:41:45 | zigo | root@C1-z-controller-1>_ ~ # openstack flavor create --ram 12288 --disk 10 --vcpus 4 cpu4-ram12-disk10 | |
| 14:41:46 | zigo | Policy doesn't allow os_compute_api:os-flavor-manage:create to be performed. (HTTP 403) (Request-ID: req-7f1c4c5b-8df2-4ef7-8a88-8f2cae1899f1) | |
| 14:41:48 | zigo | dansmith: ^ | |
| 14:42:15 | zigo | That's with the default policy.json file as per https://salsa.debian.org/openstack-team/services/nova/-/blob/debian/ussuri/debian/rules#L64 ... | |
| 14:42:24 | dansmith | I don't know why that would be, although I'm not very familiar with policy stuff | |
| 14:42:36 | dansmith | if that's the case, however, we've broken upgrade which we have to fix | |
| 14:43:00 | gmann | dansmith: generated file from oslo tool does not add the deprecated rule so nova consider those rule as override rule and only new token pass | |
| 14:43:24 | dansmith | gmann: same would go for any existing overrides the deployer has then? | |
| 14:43:29 | gmann | dansmith: if no file and rely on default then there is no cange | |
| 14:43:30 | gmann | change | |
| 14:43:50 | gmann | dansmith: if they have override all the rules then yes as they do not rely on default. | |
| 14:44:16 | gmann | oslo does not add deprecated rule if they are present in file. | |
| 14:44:22 | dansmith | gmann: the we broke upgrade for anyone using a distro's generated file from <=Ussuri right? | |
| 14:45:08 | dansmith | I'm not sure what "override all the rules" has to do with this, or why it's different than "override one rule" | |
| 14:45:13 | gmann | dansmith: "distro's generated file" is something i doubt that it is correct way or not. it is same issue they had in all the previous changed policy | |
| 14:45:40 | dansmith | gmann: doubt what is correct? | |
| 14:45:41 | zigo | dansmith: Correct ! :) | |
| 14:45:54 | gmann | dansmith: in case of "override one rule" other rule should not be in file. if they are then oslo cannot add deprecated rule | |
| 14:46:23 | dansmith | gmann: I don't understand what you're saying | |
| 14:46:31 | zigo | Also, how are operators supposed to double-guess what's currently in place, if I can't, as a package maintainer, generate what's currently in? | |
| 14:46:43 | dansmith | if they have one override in their file, everything is fine, but if they override all the rules then ...broken? | |
| 14:47:13 | zigo | If I understand correctly, the issue is to not show what's deprecated in the policy. Well, can't we simply add an option to the generator, so it also adds the deprecated things? | |
| 14:47:22 | gmann | dansmith: if they override one of all rule then rules with changed default will consider only override value not default right | |
| 14:47:31 | dansmith | zigo: presumably they look at the /usr/share version and add what they want into their file, but I think ideally we'd want a file fully commented-out where things can be uncommented and changed, but our json format probably doesn't allow that | |
| 14:47:58 | gmann | yeah, that ^^ | |
| 14:48:18 | zigo | dansmith: As much as I know, there's no way to add comments in a .json file. | |
| 14:48:21 | zigo | Indeed. | |
| 14:48:29 | dansmith | gmann: but I think zigo is saying that debian has taken the more user-friendly approach of just putting the generated file in place, and letting them alter it in-place | |
| 14:48:40 | gmann | hummm | |
| 14:48:48 | dansmith | zigo: right, it's frustrating, so I understand why the debian packages are the way they are | |
| 14:48:49 | zigo | dansmith: Exactly what I was doing so far ! :) | |
| 14:48:59 | dansmith | zigo: I'm sure you're not the only one | |
| 14:49:01 | gmann | because oslo tool generate the rule with all commented | |
| 14:49:03 | openstackgerrit | Stephen Finucane proposed openstack/nova master: objects: Add MigrationTypeField https://review.opendev.org/706013 | |
| 14:49:03 | openstackgerrit | Stephen Finucane proposed openstack/nova master: objects: Remove 'NovaObjectDictCompat' from 'Migration' https://review.opendev.org/723572 | |
| 14:49:04 | openstackgerrit | Stephen Finucane proposed openstack/nova master: objects: Remove 'NovaObjectDictCompat' from 'InstancePCIRequest' https://review.opendev.org/723573 | |
| 14:49:05 | openstackgerrit | Artom Lifshitz proposed openstack/nova stable/stein: DNM: Add a placement audit command https://review.opendev.org/720839 | |
| 14:49:17 | dansmith | I would not be surprised if people generating their own packages or installing from pip do the same for audit reasons | |
| 14:49:37 | dansmith | gmann: does it? how do you comment in json? | |
| 14:50:01 | beekneemech | It uses YAML. | |
| 14:50:25 | zigo | bnemec: As much as I know, there's no way to get services to load .yaml files, is there? | |
| 14:50:42 | bnemec | zigo: Yes, YAML works fine. | |
| 14:50:44 | zigo | Unless this has changed recently ... | |
| 14:50:47 | gmann | ah its yaml generated - https://docs.openstack.org/nova/latest/configuration/sample-policy.html | |
| 14:50:50 | dansmith | I've never seen it deployed in yaml file on a real system | |
| 14:50:51 | bnemec | I think the default is still JSON though. | |
| 14:51:11 | bnemec | IIRC, some service actually overrides that default so they get YAML by default. | |
| 14:51:13 | zigo | bnemec: Last time I tried, maybe 2 or 3 releases ago, it didn't work. | |
| 14:51:31 | zigo | Commented yaml would work for me. | |
| 14:51:44 | dansmith | zigo: except we can't require people to convert that as part of an upgrade | |
| 14:51:51 | bnemec | It's always possible there's a bug. YAML is definitely supposed to work. | |
| 14:52:03 | dansmith | bnemec: do any CI jobs use yaml? | |
| 14:53:10 | gmann | one things we can do is always add deprecated rule from oslopolicy-sample-generator | |
| 14:53:22 | dansmith | just checked one I had handy and the only service with a policy file is neutron, and it's json | |
| 14:53:38 | dansmith | gmann: but ... people with existing policy files can't be broken by this upgrade | |
| 14:53:42 | openstackgerrit | Artom Lifshitz proposed openstack/nova stable/rocky: DNM: Add a placement audit command https://review.opendev.org/720842 | |
| 14:53:56 | gmann | but again not all people use this or some other way to generate file like editing the old file | |
| 14:54:52 | gmann | dansmith: true, existing policy should not break, here zigo case is it get generated newly with oslo tool which had new defaults but not deprecated | |