| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-08-27 | |||
| 17:40:13 | gmann | https://pastebin.com/GN7mz5tE | |
| 17:40:30 | dansmith | gmann: no, that's not what we want | |
| 17:41:15 | dansmith | if it's set to the old value, then we just use the old value, end of story | |
| 17:41:35 | dansmith | if it's not overridden, then we need to check the new value, and if not present, check the old value..that's it | |
| 17:41:39 | sean-k-mooney | gmann: that woudl expand to 'policy.json'.replace('yaml', 'json') on line 14 | |
| 17:42:08 | dansmith | right | |
| 17:44:04 | gmann | in case of overridden to any new-name.json also else block is executed right? | |
| 17:44:30 | dansmith | if the value is set to *anything* other than the new default in config, then we just honor that | |
| 17:45:10 | dansmith | the ONLY thing we need to handle is the case where the value is not overridden and the operator or packager assumes the filename is the old value | |
| 17:45:22 | dansmith | i.e. "I don't need to set it in my config because I use the default filename" | |
| 17:45:33 | sean-k-mooney | gmann: did you miss the early return on line 11 https://pastebin.com/WASufpac | |
| 17:45:43 | dansmith | we break those people if we change the default filename | |
| 17:46:17 | dansmith | we we'd like to change the default filename, but still honor the old one IF-AND-ONLY-IF they're relying on the default and have not specified a value in their config file | |
| 17:46:50 | sean-k-mooney | im going to go have food o/ | |
| 17:46:51 | gmann | yeah but i was thinking to change the existing default policy file instead if cONF default based on what exist | |
| 17:47:25 | dansmith | gmann: I don't understand that last statement | |
| 17:51:28 | gmann | overall goal i was thinking is to warn with upgrade checks and change default to yaml so that oslo_policy can remove the json format support at some point. | |
| 17:51:54 | gmann | but having fallback is kind of same situation with what we have currently. | |
| 17:52:08 | dansmith | we were discussion two options and maybe we confused you, so let me reiterate the two options: | |
| 17:52:23 | dansmith | 1. Leave the default as json now, warn in nova status, change default later and break people that didn't notice | |
| 17:52:33 | gmann | yeah this one | |
| 17:52:54 | dansmith | 2. Switch default to yaml now, but fall back to json file for a release or two and warn in nova-status | |
| 17:53:29 | dansmith | if we do #2, which is my pastebin, then we can remove json more aggressively because we have been defaulting to yaml for one more release (i.e. default to yaml starts in V) | |
| 17:53:48 | dansmith | sean-k-mooney and I prefer #2, more aggressive, without breaking people, so we can say json goes away sooner | |
| 17:54:04 | gmann | but operator would not notice the default change as fallback is there. | |
| 17:54:10 | dansmith | if we do #1, then we will want to wait longer | |
| 17:54:12 | gmann | unless we fail upgrade checks | |
| 17:54:30 | dansmith | the upgrade checks can be the same in both cases | |
| 17:54:32 | gmann | i was thinking in option1, fail upgrade checks in W | |
| 17:55:02 | dansmith | gmann: we can fail upgrade checks in V if we do #2, we just need to fall back to the old filename because not everyone runs upgrade checks | |
| 17:55:53 | gmann | humm, you mean fail upgrade check but also fallback? does not that make upgrade checks a false info? | |
| 17:56:23 | gmann | upgrade checks should fail at the same time we start rejecting the json file in both otion | |
| 17:56:25 | gmann | option | |
| 17:56:56 | dansmith | the upgrade checks are informational anyway.. we have "warning" and "failure" in upgrade checks, we can warn now and fail on the release where we remove it in either case | |
| 17:57:34 | gmann | yeah fail later seems more correct and what our code will behave | |
| 17:57:35 | dansmith | if we do #2, then we get all new users to yaml right now, which means we can dump json support sooner | |
| 17:57:47 | dansmith | that's what we're going for | |
| 17:57:54 | dansmith | it doesn't change the signaling now at all | |
| 17:58:02 | dansmith | it just means we can get all new users on yaml right away, | |
| 17:58:05 | gmann | yeah that is good i think. | |
| 17:58:15 | dansmith | instead of new users being on yaml later when everyone else if forced to do the same | |
| 17:58:48 | gmann | yeah. it is better way | |
| 18:00:10 | gmann | and this warning in doc still valid for option2 also - https://review.opendev.org/#/c/748059/1/doc/source/configuration/policy-concepts.rst | |
| 18:00:29 | gmann | or we should add default value things also here? | |
| 18:01:03 | dansmith | warning applies to either for sure, I wouldn't add the json fallback here | |
| 18:01:16 | gmann | ok | |
| 18:01:18 | dansmith | I would only mention the fallback in the reno | |
| 18:02:19 | gmann | will update patch with option2 once oslo.policy is released. thanks dansmith sean-k-mooney . | |
| 18:08:15 | dansmith | cool, thanks gmann ! | |
| 18:08:27 | gmann | dansmith: but i will add "elif CONF.policy_file == OLD_POLICY_FILE:" for case - policy.json exist but an old, not supposed to be used file and CONF.policy_file =nova_policy.* means overridden file | |
| 18:09:07 | dansmith | gmann: then I will -1 it :) | |
| 18:09:13 | gmann | so that we do not pick not-supposed-to-use but existing policy.json | |
| 18:09:15 | gmann | :) | |
| 18:09:30 | dansmith | if CONF.policy_file is set to anything, then we should do what they ask | |
| 18:10:04 | dansmith | but go ahead and propose what you want and let sean-k-mooney and I have a chance to -1 with discussion and we can do it there, which might be easier since we can reference actual code and lines | |
| 18:10:59 | gmann | but where you are checking override file? | |
| 18:11:04 | gmann | ok | |
| 18:13:12 | dansmith | bnemec: apologize for asking this again, but.. there's some oslo_config method for determining if the conf option has been set or not right? | |
| 18:13:59 | mriedem | set vs the value coming from a default? | |
| 18:14:00 | dansmith | bnemec: is it just opt.value == opt.default ? | |
| 18:14:06 | dansmith | mriedem: yeah | |
| 18:14:12 | mriedem | there is some location thing i thought | |
| 18:14:19 | mriedem | which says where the value comes from | |
| 18:14:48 | mriedem | https://github.com/openstack/oslo.config/blob/master/oslo_config/cfg.py#L57 | |
| 18:14:58 | bnemec | Yes | |
| 18:15:06 | dansmith | okay, perfect | |
| 18:15:34 | bnemec | Docs are here: https://docs.openstack.org/oslo.config/latest/reference/locations.html | |
| 18:15:41 | dansmith | gmann: for sean-k-mooney and my desired behavior, the *only* thing you need to check is if CONF.policy_file's location == opt_default :) | |
| 18:15:43 | gmann | +1, that can save any accidental pick of file if both extensions exist | |
| 18:15:52 | dansmith | right | |
| 18:16:49 | gmann | +1. CONF.policy_file's location in [opt_default, set_default ] as nova will do set_default to change the default. | |
| 18:18:59 | dansmith | ack | |
| 18:39:06 | openstackgerrit | Lee Yarwood proposed openstack/nova stable/stein: Improve CinderFixtureNewAttachFlow https://review.opendev.org/748509 | |
| 18:39:07 | openstackgerrit | Lee Yarwood proposed openstack/nova stable/stein: Robustify attachment tracking in CinderFixtureNewAttachFlow https://review.opendev.org/748510 | |
| 18:39:08 | openstackgerrit | Lee Yarwood proposed openstack/nova stable/stein: compute: Use source_bdms to reset attachment_ids during LM rollback https://review.opendev.org/748511 | |
| 18:39:09 | openstackgerrit | Lee Yarwood proposed openstack/nova stable/stein: compute: refactor volume bdm rollback error handling https://review.opendev.org/748512 | |
| 18:39:11 | openstack | bug 1889108 in OpenStack Compute (nova) train "failures during driver.pre_live_migration remove source attachments during rollback" [Undecided,In progress] https://launchpad.net/bugs/1889108 - Assigned to Lee Yarwood (lyarwood) | |
| 18:39:11 | openstackgerrit | Lee Yarwood proposed openstack/nova stable/stein: WIP Add regression tests for bug #1889108 https://review.opendev.org/748513 | |
| 18:39:12 | openstackgerrit | Lee Yarwood proposed openstack/nova stable/stein: compute: Don't delete the original attachment during pre LM rollback https://review.opendev.org/748514 | |
| 18:54:30 | lyarwood | melwitt: https://review.opendev.org/#/c/742415/ - can you look at this if you get time, it's holding up the CVE on stable/train | |
| 18:54:49 | lyarwood | melwitt: well the release at least | |
| 19:03:29 | openstack | Launchpad bug 1893263 in OpenStack Compute (nova) "Cannot create instance with multiqueue image and vif_type=tap (calico)" [Undecided,New] | |
| 19:03:29 | ganso | hello Nova folks! I just logged bug: https://bugs.launchpad.net/nova/+bug/1893263 after reproducing the issue on master. I am planning to work on the fix myself, however, I am curious to know if anyone here knows the reason why plug_tap() needs to run, as if the instance is ever being rebooted, the tap device goes away and is re-created by libvirt instead (therefore plug_tap() seems to have never needed to run at all) | |
| 19:04:08 | ganso | because one of the possible fixes is just to remove plug_tap() and be happy | |
| 19:07:49 | openstack | bug 1889108 in OpenStack Compute (nova) train "failures during driver.pre_live_migration remove source attachments during rollback" [Undecided,In progress] https://launchpad.net/bugs/1889108 - Assigned to Lee Yarwood (lyarwood) | |
| 19:07:49 | openstackgerrit | Lee Yarwood proposed openstack/nova stable/train: Add regression tests for bug #1889108 https://review.opendev.org/748494 | |
| 19:07:50 | openstackgerrit | Lee Yarwood proposed openstack/nova stable/train: compute: Don't delete the original attachment during pre LM rollback https://review.opendev.org/748496 | |
| 19:07:50 | openstackgerrit | Lee Yarwood proposed openstack/nova stable/train: compute: refactor volume bdm rollback error handling https://review.opendev.org/748495 | |
| 19:11:33 | openstack | bug 1889108 in OpenStack Compute (nova) train "failures during driver.pre_live_migration remove source attachments during rollback" [Undecided,In progress] https://launchpad.net/bugs/1889108 - Assigned to Lee Yarwood (lyarwood) | |
| 19:11:33 | openstackgerrit | Lee Yarwood proposed openstack/nova stable/stein: Add regression tests for bug #1889108 https://review.opendev.org/748513 | |
| 19:11:34 | openstackgerrit | Lee Yarwood proposed openstack/nova stable/stein: compute: Don't delete the original attachment during pre LM rollback https://review.opendev.org/748514 | |
| 19:26:05 | melwitt | lyarwood: ack will do | |
| 19:30:22 | lyarwood | melwitt: thanks! :) | |
| 19:31:32 | lyarwood | artom: https://review.opendev.org/#/c/747451/ - LGTM, I've got a few questions but I'm not comfortable +2+W'ing networking stuff so even with these answered I'm going to defer to other cores on this one | |
| 19:31:55 | melwitt | sean-k-mooney: fyi I added you to review https://review.opendev.org/747451 please. iirc you have said using the cache is fine but it would be good if you could ack it | |
| 19:32:14 | lyarwood | ah there we go \o/ | |
| 20:16:57 | openstackgerrit | melanie witt proposed openstack/nova master: Change default num_retries for glance to 3 https://review.opendev.org/740389 | |
| 20:37:26 | openstackgerrit | Rodrigo Barbieri proposed openstack/nova master: Allow tap interface with multiqueue https://review.opendev.org/748533 | |
| 21:57:54 | sean-k-mooney | melwitt: yes, when using multiple port bindings the ports are still active on the source host at this point and when not using multiple portbining we update the neutron port later in post_live_migration_at_destination | |
| 21:59:22 | sean-k-mooney | melwitt: there are other point in pre live migration where it would also be valid to use the cache for the same reason | |