Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2020-01-30
16:59:39 melwitt yeah, I assumed that's why it's still around but tbh I have never dug in more
16:59:52 bauzas it would be easier to pull the trigger and convince people after that
17:00:00 stephenfin melwitt: So come up with a osc-placement replacement for the 'openstack hypervisor show' command?
17:00:23 bauzas I can't recollect in my mind but I remember we had a convo around it in some f2f like summit or ptg
17:00:32 bauzas stephenfin: document it first
17:00:44 bauzas and then ask whether ops are okay with the replacement as it is
17:01:01 bauzas (that'd be my recommendation in order not to freak out ops)
17:01:05 melwitt stephenfin: no sorry, I was just saying if it takes multiple calls to placement to gather the same data that os-hypervisors gives in one call (I don't know if it does, but if this is the case) then consider a osc-placement cli batch command to give operators that ease that they had before with os-hypervisors
17:01:16 stephenfin Ah, gotcha
17:02:12 melwitt ++ yeah this is all hypothetical until you dig in, just wanted to cover the considerations at a high level
17:09:08 openstackgerrit Merged openstack/nova master: zuul: Remove unnecessary 'USE_PYTHON3' https://review.opendev.org/695380
17:12:15 gibi dansmith: if you have time, I replied to your comments in https://review.opendev.org/#/c/702261
17:12:59 efried dustinc: are you planning to do anything further with https://review.opendev.org/#/q/topic:bp/openstacksdk-in-nova+status:open ?
17:15:28 dansmith gibi: okay I'll have to rebuild context on it
17:17:41 dansmith gibi: are you saying that we've received an instance with a modified pci request over rpc from the destination and we're doing the refresh to dump that and avoid persisting the changed-but-no-longer-relevant field on the instance?
17:19:33 gibi dansmith: exactly
17:19:38 dansmith if so, refresh seems like a big hammer for that because it will drop any other changes we have on the instance that we might want (even if that happens in the future). Looks like refresh can't take a list of fields (which I guess makes sense) so I don't really have an alternative
17:21:21 gibi dansmith: right now I don't feel we have a problem with refresh, but if it happens in the future then we need to extend the ovo interface or start saveing the old value of the pci request somewhere
17:21:38 dansmith refresh isn't an ovo thing, it's an instance thing
17:21:48 efried sean-k-mooney: You still planning to drive https://review.opendev.org/#/q/topic:bp/image-metadata-prefiltering+status:open in ussuri?
17:21:49 gibi dansmith: ohh, then I can hack on it right now
17:22:15 dansmith gibi: what about checking obj_what_changed() right before the refresh and asserting that it's either empty or just contains the pci request info and logging a warning if not?
17:23:07 dansmith gibi: in fact if you did that you could avoid the expensive refresh for everyone else if it's not SRIOV
17:23:14 gibi dansmith: I can do that. But I can also extend the refresh() call with an optiona; field list
17:23:45 dansmith gibi: I'm less excited about the latter just because of the potential effort in validating it, but it seems like that might be useful
17:24:21 dansmith gibi: I think the reason we didn't initially do that is because you may be creating a franken-instance where you've pulled some updates from the db and not others, which are co-dependent and then would save it back in an inconsistent state
17:24:22 gibi dansmith: OK. I will follow your suggestion and check obj_what_changed and log a warning if we would drop other fields than pci
17:24:37 dansmith gibi: and avoid the refresh if nothing is changed yeah?
17:24:41 gibi yepp
17:24:43 dansmith cool
17:24:50 gibi and make a note about selective refresh for later
17:24:57 gibi dansmith: thanks!
17:25:15 dansmith ack, also note my piconit :)
17:25:53 gibi ack :)
17:28:58 gibi o/
17:41:01 gmann cmurphy: can you revisit this review. i replied in the comment, https://review.opendev.org/#/c/701624/
17:41:43 gmann doing deprecation in base rule is to avoid adding the deprecation warning in each rule using base rule as their check_str
17:44:17 gmann cmurphy: you can find more discussion here on doing deprecation in base rule than specific rules - https://review.opendev.org/#/c/676682/4/nova/policies/admin_actions.py
17:49:14 efried stephenfin: is there anything further to be done for https://blueprints.launchpad.net/nova/+spec/remove-xvpvncproxy or can I mark it complete?
17:50:21 stephenfin efried: Nope, melwitt and mriedem closed it out a few weeks back. It's done.
17:50:27 efried sweet
17:51:59 cmurphy gmann: i will come back to it today, thanks for pointing me to the discussion
17:52:19 cmurphy i'm hoping lbragstad could also offer some insights ^
17:52:21 gmann cmurphy: thanks
17:52:25 gmann +1
18:26:15 efried dansmith: Are you pursuing this https://review.opendev.org/#/c/693226/ ? (nova-audit)
18:26:33 efried If so, would you please throw out a bp for it? I'm getting those ducks lined up.
18:28:10 dansmith efried: no, I had abandoned it
18:28:31 efried oh, looks like melwitt restored it. melwitt ^ ?
18:29:16 melwitt I had restored it to encourage ops review, I was told the abandonedness confused the people who would comment on it
18:30:02 efried melwitt: k. Been a couple weeks. If you wouldn't mind putting up a blueprint so it's properly trackable. Then I guess we can go-no-go it at spec freeze time.
18:30:36 melwitt efried: yeah, makes sense. I will do that
18:37:53 openstackgerrit Ghanshyam Mann proposed openstack/nova master: Deprecate base rules in favor of new rules https://review.opendev.org/701624
18:38:23 gmann cmurphy: lbragstad done ^^
19:22:13 openstackgerrit Merged openstack/nova master: Use COMPUTE_SAME_HOST_COLD_MIGRATE trait during migrate https://review.opendev.org/695220
19:22:18 openstackgerrit Merged openstack/nova stable/stein: Remove 'test_cold_migrate_with_physnet_fails' test https://review.opendev.org/702971
19:34:59 openstackgerrit Artom Lifshitz proposed openstack/nova master: Functional tests for NUMA live migration https://review.opendev.org/672595
19:35:01 openstack bug 1845146 in OpenStack Compute (nova) train "NUMA aware live migration failed when vCPU pin set" [High,Fix committed] https://launchpad.net/bugs/1845146 - Assigned to Dan Smith (danms)
19:35:01 openstackgerrit Artom Lifshitz proposed openstack/nova master: NUMA LM: Add func test for bug 1845146 https://review.opendev.org/687404
19:52:08 stephenfin sean-k-mooney: want to drop your -W from https://review.opendev.org/#/c/703115/ now that the stable/stein changes have merged
19:53:32 sean-k-mooney done
20:57:11 efried Nova meeting shortly in #openstack-meeting
21:48:51 melwitt efried: created https://blueprints.launchpad.net/nova/+spec/nova-audit
21:49:03 efried thanks melwitt
22:16:57 openstackgerrit Ghanshyam Mann proposed openstack/nova master: Add test coverage of existing attach_interfaces policies https://review.opendev.org/705126
22:40:42 openstackgerrit Ghanshyam Mann proposed openstack/nova master: Remove old policy enforcement in attach_interfaces https://review.opendev.org/705127
22:52:57 openstackgerrit Ghanshyam Mann proposed openstack/nova master: Add test coverage of existing attach_interfaces policies https://review.opendev.org/705126
22:55:15 openstackgerrit Ghanshyam Mann proposed openstack/nova master: Add test coverage of existing attach_interfaces policies https://review.opendev.org/705126
23:14:11 openstackgerrit Ghanshyam Mann proposed openstack/nova master: Add test coverage of existing attach_interfaces policies https://review.opendev.org/705126
23:40:03 melwitt gmann: question on https://review.opendev.org/701629
23:45:13 gmann melwitt: replied. I am removing user_id from policy target in https://review.opendev.org/#/c/701642/
23:45:49 gmann melwitt: but if we keep this operation restriction at user level then we can keep.
23:46:32 gmann we do not pass user_id for most of the(i think all except user level API like keypair etc) API. it is all enforced at project level
23:46:36 melwitt gmann: ok. I'm thinking from the perspective of us having to keep the legacy policy behavior for some time or some transition period. is my understanding correct?
23:47:22 melwitt because from the look of the old tests, we support letting operators restrict password change to only the user set on the instance
23:48:36 gmann melwitt: yeah, that is good point. We can keep supporting it same way till we completely migrate to new
23:49:06 melwitt I'm happy to be wrong but I thought we were keeping current support as-is for now and if we want to change it, need some signal about deprecation and a transition period before dropping
23:50:17 melwitt I was reading this part of the spec again https://specs.openstack.org/openstack/nova-specs/specs/ussuri/approved/policy-defaults-refresh.html#backward-compatibility-and-migration-plan
23:52:40 gmann yeah. by removing the user_id from target we can break some deployment.
23:52:46 melwitt "Remove any project or user checks from the policy file defaults, as this is now done in code, without breaking user-id-based-policy-enforcement"
23:53:01 melwitt I'm wondering if we removed any in the already merged patches
23:54:30 gmann no, this is first one where user_id is passed and i updated to remove that. in "pass actual target" patches I pass the actual target wherever needed. like owner rule where project_id is needed.
23:55:22 melwitt ok
23:55:48 gmann "pass actual target" patches can tell us if any where we are changing user-level enforcement
23:56:04 melwitt ok, that is good to know
23:57:00 melwitt not directly related to this, I do wonder if operators would be disappointed to lose user-level enforcement in the future on something like a admin password. but that's a separate issue
23:58:15 gmann yeah, i think we need to change that with separate discussion if everyone ok. else keep it as it is.
23:58:15 melwitt I see now in the pass actual target, thanks for the tip https://review.opendev.org/#/c/701642/1/nova/api/openstack/compute/admin_password.py
23:59:25 gmann melwitt: i found one bug in attach_interface policy which is admin_or_owner but it pass for every context because project_id is not passed in policy target
23:59:29 melwitt yeah, agreed. I can't predict what operators would want to see there, would be a good question for the forum and if no one cares about user-level enforcement, we remove it after the transition period
#openstack-nova - 2020-01-31
00:00:20 melwitt gmann: bug in the legacy policy or bug in the more recently policy patches merged?
00:00:20 gmann because our policy doc say it is admin_or_owner but code is buggy so I am thinking whether we can fix this as bug or leave this operation open for everyone
00:00:27 gmann legacy
00:00:58 gmann this one - https://review.opendev.org/#/c/705126/4
00:00:58 melwitt sigh ... I'm not surprised, there's a lot of bugs/quirks there.
00:01:26 gmann i added the test with admin_or_owner context to pass and other proejct to fail. but those tests fail as you can see in gate.
00:01:38 melwitt I guess my instinct would be to fix the bug first and then stack the new stuff on top, but that might be just me
00:02:24 gmann yeah that is what i was thinking. so that we can see the bug and fix first and then start the new policy defaults
00:02:25 melwitt I see. well, that's a good sign for the new test coverage I guess :)
00:02:32 gmann yeah

Earlier   Later