Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-11-26
21:19:40 artom mriedem, does the superconductor really rescheduler if the live migrations fails? I'm looking but I can't find anything in _execute, and in the conductor manager if there's a failure in _live_migrate it just sets an error.
21:20:00 artom Not sure it's super relevant to the spec, but for my own personal edification
21:21:39 mriedem mnaser: you mean drop it at this point?
21:22:19 mnaser mriedem: i think so? i mean it's just dead code for 5 years, do we want to muck around with microversion bumps and blah
21:23:25 mriedem mnaser: tbc, you're saying just ditch the commented out code since no one cares enough to change it with a new microversion
21:23:39 mnaser yep
21:24:29 mriedem shrug
21:24:33 mriedem i don't see anyone caring enough to change it
21:24:54 mriedem the fact you have to supply the all_tenants parameter to filter on project_id does always confuse me
21:25:00 mnaser well
21:25:03 mnaser the clients workaround it now..
21:25:03 mriedem but at least it's documented in the api-ref
21:25:13 mnaser pretty sure you dont need to do that anymore with the cli
21:25:40 mriedem https://docs.openstack.org/python-novaclient/latest/cli/nova.html#nova-list
21:25:48 mriedem nova list --tenant will just implicitly add --all-tenants
21:25:59 mriedem if that's what you mean by workaround
21:26:20 mnaser yeah
21:26:41 mnaser https://github.com/openstack/python-openstackclient/blob/master/openstackclient/compute/v2/server.py#L1147-L1153
21:26:44 mnaser same for osc
21:27:20 mriedem if you want to push a patch to remove the cruft, fine by me
21:27:23 mriedem i might even +2 that
21:27:52 mriedem anything to make that method smaller b/c god is it long
21:35:23 mriedem bnemec: have you ever heard of requests for something like a PostitiveIntOpt or PositiveFloatOpt in oslo.config? we have some options which can be set to 0.0 as the min, but really shouldn't be <= 0.
21:35:32 mriedem but we can't really describe that with just min
21:37:27 dansmith choices=range(1,1000) ? :P
21:37:36 bnemec mriedem: So an opt where min is a < comparison instead of a <=?
21:38:21 mriedem something like that
21:38:28 mriedem for context https://review.openstack.org/#/c/602804/9/nova/conf/compute.py
21:38:36 mriedem initial_cpu_allocation_ratio should never be 0.0
21:38:39 dansmith we really just want a validation function parameter, right?
21:38:43 mriedem yeah
21:38:44 dansmith we wanted that for something else recently
21:38:53 dansmith validator=lambda str: ...
21:38:53 openstackgerrit Mohammed Naser proposed openstack/nova master: Drop cruft code for all_tenants behaviour https://review.openstack.org/620165
21:41:19 bnemec A validator callback seems like something we could do.
21:41:32 bnemec Alternatively, in this case min=0.000001 is probably also sane.
21:46:16 bnemec You could also create a custom type that did the validation in the constructor.
21:46:44 bnemec Subclass Float and put whatever logic you need in there: https://github.com/openstack/oslo.config/blob/master/oslo_config/types.py#L409
21:47:14 mriedem I wasn't sure how kosher subclassing oslo.config opt types was
21:47:16 bnemec Then create the opt as Opt(type=MyCustomType, ...).
21:47:55 bnemec They're part of the public API so I'd say they're fair game.
21:49:20 mriedem ok yeah that's probably cleanest
21:49:25 bnemec They danger might be creating a completely new class as a type, which could theoretically be done, but if we ever added to the type API you might get broken.
21:49:32 bnemec As long as you inherit from an existing type you should be okay though.
21:49:57 bnemec They all descend from a single ABC.
21:50:53 mriedem the adam of config opts?
21:51:14 bnemec Indeed.
21:51:30 bnemec Also, I think I was wrong. You want to do validation in __call__.
21:51:40 bnemec That's where we're doing it in the existing types: https://github.com/openstack/oslo.config/blob/master/oslo_config/types.py#L830
21:52:19 mriedem ah yeah https://github.com/openstack/oslo.config/blob/master/oslo_config/types.py#L305
21:52:51 bnemec Yeah, better example. :-)
22:03:19 mriedem welp https://review.openstack.org/#/c/613126/ kind of blows up https://specs.openstack.org/openstack/nova-specs/specs/stein/approved/initial-allocation-ratios.html#manually-set-placement-allocation-ratios-are-overwritten
22:03:23 mriedem how am i not surprised
22:05:21 mriedem efried: you'll have some context on ^
22:06:52 mriedem i'm not exactly sure what we should do about it, outside of passing an initial flag to update_provider_tree or something gross like that...although maybe upt can check the provider tree to see if it already has inventory with allocation_ratio set, and if so, don't provide a value unless CONF.*_allocation_ratio is not None
22:09:27 openstackgerrit Artom Lifshitz proposed openstack/nova master: Give drop_move_claim() correct docstring https://review.openstack.org/620170
22:09:43 artom mriedem, ^^ related to the numa live migration spec. I *think* I'm right, and it might clarify the confusion around where we're removing usages.
22:10:31 mriedem artom: totally forgot you pinged me earlier, sec
22:10:52 mriedem artom: superconductor does not reschedule if live migration fails, no
22:10:59 mriedem it reschedules if the pre-checks on the dest/source fail
22:11:34 artom Right, sorry, I wasn't being precise enough. Though I can't find it rescheduling *anywhere*
22:11:45 mriedem so here conductor asks the scheduler for a host https://github.com/openstack/nova/blob/master/nova/conductor/tasks/live_migrate.py#L370
22:12:01 mriedem in this try block it does the pre-checks for dest/source https://github.com/openstack/nova/blob/master/nova/conductor/tasks/live_migrate.py#L385
22:12:15 mriedem if that fails, blacklist the host https://github.com/openstack/nova/blob/master/nova/conductor/tasks/live_migrate.py#L391
22:12:16 artom *facepalm*
22:12:21 artom Got it, sorry.
22:12:28 mriedem https://github.com/openstack/nova/blob/master/nova/conductor/tasks/live_migrate.py#L366
22:12:28 mriedem set host=None and hit the while again
22:12:38 mriedem np
22:12:42 artom My brain skipped over _find_destination() as an atomic thing
22:13:32 artom Thanks for taking the time to walk me through this :)
22:14:00 artom Now that comment patch, if you have time ;) The patch itself is trivial, but requires thinking to make sure I got it right.
22:14:07 openstackgerrit Merged openstack/nova master: Add debug logs when doubling-up allocations during scheduling https://review.openstack.org/617016
22:14:14 openstackgerrit Merged openstack/nova stable/rocky: Default embedded instance.flavor.is_public attribute https://review.openstack.org/619349
22:16:45 openstackgerrit Artom Lifshitz proposed openstack/nova master: Give drop_move_claim() correct docstring https://review.openstack.org/620170
22:31:14 mriedem artom: ok some random comments inline
22:31:40 mriedem i guess drop_move_claim called from the source on confirm_resize always confuses me,
22:31:47 mriedem because the claim is made on the dest during prep_resize
22:31:52 efried mriedem: Sorry, I'm back now. We're trying to figure out how to know whether allocation ratios were defaulted by placement or conf or nova?
22:32:57 mriedem efried: yeah - need to determine when initial_*_allocation_ratio config should be used (first time the compute was created) and report that to placement, or if we should set allocation ratios because CONF.*_allocation_ratio is some non-default value, or if we should leave it alone because the admin set some allocation_ratio via the placement API directly
22:33:24 mriedem i think we can know the initial case from upt if the provider tree doesn't have those inventory resource class keys in it
22:33:44 mriedem otherwise only ever overwrite if CONF.*_allocation_ratio is not None
22:34:05 efried "if the provider tree doesn't have those inventory resource class keys" <== I don't think this can happen within upt
22:34:30 mriedem on initial startup, wont upt just have a root provider with no inventory on it?
22:34:35 mriedem the driver reports the inventory initially
22:34:41 efried I don't think so, because of the (legacy) code that's bootstrapping the CPU/MEMORY_MB/DISK_GB inventories before we get to upt
22:35:03 mriedem i'm not sure what you're referring to
22:35:06 efried I could be wrong, it's possible we've removed that at this point.
22:35:35 efried but there was a time when code *outside* of virt driver space would do the initial populate of placement
22:35:52 mriedem by calling driver.get_inventory() right?
22:36:04 mriedem that no longer happens if the driver implements upt
22:36:05 efried get_available_resource
22:36:14 efried which *does* still get called I believe.
22:36:14 mriedem yeah, get_available_resource was before get_inventory
22:36:33 mriedem i think it's upt -> get_inventory -> get_available_resource
22:36:37 efried gar doesn't get called in the rt update flow, but it still gets called somewhere else, lemme find it.
22:37:03 mriedem https://github.com/openstack/nova/blob/c1de096098344c733565c244163fc3ebf8c35e68/nova/compute/resource_tracker.py#L711
22:37:50 mriedem we'd create the provider root initially here https://github.com/openstack/nova/blob/c1de096098344c733565c244163fc3ebf8c35e68/nova/compute/resource_tracker.py#L921
22:38:05 mriedem then flush inventory from upt here https://github.com/openstack/nova/blob/c1de096098344c733565c244163fc3ebf8c35e68/nova/compute/resource_tracker.py#L944
22:38:13 mriedem else this https://github.com/openstack/nova/blob/c1de096098344c733565c244163fc3ebf8c35e68/nova/compute/resource_tracker.py#L951

Earlier   Later