Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-09-05
19:28:00 dansmith anyone else could land a patch right now that calls that code in such a way that it won't generate a sensible placement request
19:28:07 dansmith and since it's kinda library/client code, it should be landed in a way that makes it work if called, IMHO
19:28:35 efried ack
19:28:36 dansmith so, to summarize,
19:28:41 dansmith if it were me, I would ask for:
19:29:03 dansmith 1. A patch to fix up all the "oops I forgot" stuff in the placement client section, which is a chunk of the last patch currently
19:29:12 dansmith 2. A patch to add the filter and its knob
19:29:42 dansmith 3. Docs (and reno) at the end, which is the remaining bit of the last patchI think (if you subtract some of the random changes that are in there like renaming a test)
19:30:14 mriedem artom: some comments/questions on patches 2 and 3, onto the big one https://review.opendev.org/#/c/634606/
19:31:04 artom mriedem, cheers! I have to pick up kids from school in 15, so my responses will have to wait later tonight, and probably into tomorrow morning
19:32:20 efried dansmith: okay. I think I could probably do that shuffle and still +2 without losing sleep.
19:32:37 dansmith efried: agree
19:35:42 mriedem artom: yeah nothing major so far, just +1 with "i'm still going, but answer before i +2"
19:37:11 dansmith mriedem: presume you've seen the CI job logs that shows this working too.. it's handy to trawl that looking for things to line up
19:37:26 dansmith and also, sean-k-mooney did some manual testing yesterday that was meaningful to me, with edge cases and such
19:38:06 artom "+1 I'm still going" is all we can hope from life
19:43:50 mriedem dansmith: yes i was https://zuul.opendev.org/t/openstack/build/d51f91efa937411a9179b930eff3ab08/log/controller/logs/screen-n-cpu.txt.gz#2199
19:44:50 efried rebase only...
19:44:53 openstackgerrit Eric Fried proposed openstack/nova master: Nova object changes for forbidden aggregates request filter https://review.opendev.org/671072
19:44:53 openstackgerrit Eric Fried proposed openstack/nova master: DB API changes to get non-matching aggregates from metadata https://review.opendev.org/671074
19:44:54 openstackgerrit Eric Fried proposed openstack/nova master: Add a new request filter to isolate aggregates https://review.opendev.org/671075
19:44:54 openstackgerrit Eric Fried proposed openstack/nova master: Enable request filter isolate_aggregates https://review.opendev.org/667952
19:47:17 mriedem dansmith: there is no up-call in this https://review.opendev.org/#/c/656594/
19:47:22 mriedem that code is only ever called from the api
19:47:49 dansmith mriedem: is it?
19:48:00 dansmith I thought we called down to the compute which did this
19:48:04 mriedem no
19:48:42 mriedem maybe nova-net, idk about that, nor really care about nova-net
19:49:05 mriedem this is just a proxy call from nova-api to neutron
19:49:08 dansmith sure, I just thought we still had to do FIP associate on the compute, like for LB or something
19:49:26 mriedem http://codesearch.openstack.org/?q=network_api%5C.associate_floating_ip&i=nope&files=&repos=
19:49:31 mriedem i guess mogan cares
19:50:43 dansmith okay, well, I did say I hadn't chased all the plumbing
19:51:15 dansmith I think the thing that led me there, aside from artom's comment,
19:51:34 dansmith was that you're saying "if we don't have access to the API DB" which can't happen without nothing else working, if it's just in the api
19:52:00 dansmith and the log message to that effect
19:52:43 dansmith the other change you reference was to handle calling code that could be either api-level or inside the cell
19:52:59 dansmith but if this is only ever at the API, then I don't think there's any need to catch that situation
19:53:05 dansmith and if you do,
19:53:10 dansmith it's not info-level, it's error-level,
19:53:20 dansmith along with all the other tracebacks the api would be throwing if you even got that far :)
19:56:43 mriedem ok, like i said, the CantStartEngineError thing came up in denver i think, but it's not written down so i can't remember the details for sure - i don't need it, and agree it shouldn't happen - and if it does, we're more f'ed than just this, so i'm happy to remove that in a follow up when i fix that comment in the test
19:57:11 mriedem or, if i'm backporting, just nack it and i can remove that now
19:57:28 dansmith ack, commenting
19:58:55 dansmith mriedem: done. left my +2, you can decide to fix or fup
19:59:07 dansmith for backport, I'd think it's worth fixing before it lands, since it's like 24 hours away :)
20:04:12 mriedem to be clear,
20:04:14 mriedem "So, if you're going to catch it and log, I'd say do so at error and then re-raise the exception like all the other API code would."
20:04:18 mriedem you're OK with just removing that catch right?
20:05:16 dansmith yes, better to just remove it, imho
20:05:27 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: Find instance in another cell during floating IP re-association https://review.opendev.org/656594
20:06:38 dansmith mriedem: you doing it now? I'll stand by to fast approve if so
20:07:35 mriedem no, still going through this big numa lm patch
20:07:44 mriedem just got through the commit message...
20:08:47 openstackgerrit Eric Fried proposed openstack/nova master: Nova object changes for forbidden aggregates request filter https://review.opendev.org/671072
20:08:48 openstackgerrit Eric Fried proposed openstack/nova master: DB API changes to get non-matching aggregates from metadata https://review.opendev.org/671074
20:08:48 openstackgerrit Eric Fried proposed openstack/nova master: Add a new request filter to isolate aggregates https://review.opendev.org/671075
20:08:49 openstackgerrit Eric Fried proposed openstack/nova master: Docs for isolated aggregates request filter https://review.opendev.org/667952
20:09:11 efried dansmith: gotta go on next chauffeur run, but I think that's what we were shooting for ^
20:10:16 efried basically half of the .py changes needed to be in the first patch, the other half in the third.
20:11:12 efried guess I should have done that field rename while I was in there, sec...
20:15:53 openstackgerrit Eric Fried proposed openstack/nova master: Nova object changes for forbidden aggregates request filter https://review.opendev.org/671072
20:15:53 openstackgerrit Eric Fried proposed openstack/nova master: DB API changes to get non-matching aggregates from metadata https://review.opendev.org/671074
20:15:58 openstackgerrit Eric Fried proposed openstack/nova master: Add a new request filter to isolate aggregates https://review.opendev.org/671075
20:15:58 openstackgerrit Eric Fried proposed openstack/nova master: Docs for isolated aggregates request filter https://review.opendev.org/667952
20:16:13 mriedem totally unrelated to the numa live migration stuff, but can this fail the 'claim' for sriov pci devices on the dest host? https://github.com/openstack/nova/blob/f7f5e1846c7b19aa05817df7f3c4345819db413f/nova/compute/manager.py#L6491
20:16:16 mriedem and if so, how does it fail?
20:17:07 mriedem because there is a very specific set of exceptions that will trigger a reschedule to an alternate host https://github.com/openstack/nova/blob/f7f5e1846c7b19aa05817df7f3c4345819db413f/nova/conductor/tasks/live_migrate.py#L482
20:17:23 mriedem so if that stuff doesn't result in MigrationPreCheckError we'll fail the live migratoin b/c the first host claim failed
20:17:26 mriedem sean-k-mooney: ^
20:18:41 openstackgerrit Dan Smith proposed openstack/nova master: Find instance in another cell during floating IP re-association https://review.opendev.org/656594
20:18:44 mriedem trying to follow that pci claim code is going down a rabbit hole
20:18:51 dansmith mriedem: ^
20:20:50 mriedem want to hit that test comment while you're updating?
20:43:26 openstackgerrit Merged openstack/nova master: neutron: refactor nw info cache refresh out of associate_floating_ip https://review.opendev.org/678300
20:56:31 mriedem oooo https://github.com/openstack/nova/blob/f7f5e1846c7b19aa05817df7f3c4345819db413f/nova/scheduler/filter_scheduler.py#L173-L187 - if it weren't for USES_ALLOCATION_CANDIDATES we could burn all of that queens compat code out of the filter scheduler
20:57:51 mriedem artom: dansmith: ok comments in the big one https://review.opendev.org/#/c/634606/75
21:05:40 openstackgerrit Matt Riedemann proposed openstack/nova master: Find instance in another cell during floating IP re-association https://review.opendev.org/656594
21:05:45 dansmith mriedem: cool thanks
21:05:58 mriedem ok i think that multi-cell floating ip thing is ready
21:06:33 artom mriedem, thanks! It's supper time now, will work a bit after, then gym, so tomorrow morning
21:06:43 artom Haven't been to gym in, like, a week, not cool
21:08:39 openstackgerrit Merged openstack/nova master: Trap and log errors from _update_inst_info_cache_for_disassociated_fip https://review.opendev.org/678301
21:13:32 mriedem gerrit is down anyway
21:21:18 openstackgerrit Matt Riedemann proposed openstack/nova master: Remove old comments about caching scheduler compat https://review.opendev.org/680521
21:36:11 openstackgerrit Eric Fried proposed openstack/nova master: Add a new request filter to isolate aggregates https://review.opendev.org/671075
21:36:11 openstackgerrit Eric Fried proposed openstack/nova master: Docs for isolated aggregates request filter https://review.opendev.org/667952
21:37:13 mriedem alex_xu: can you take a look at the api change to attach a volume (and list/show vol attachments) with delete_on_termination? https://review.opendev.org/#/c/673133/ - it's probably going to be 2.79; i'm +2 on it
21:42:59 openstackgerrit Adam Spiers proposed openstack/nova master: Ensure non-q35 machine type is not used when booting with SEV https://review.opendev.org/680065
21:43:32 openstackgerrit Adam Spiers proposed openstack/nova master: Extract fake KVM guest fixture for reuse https://review.opendev.org/680526
21:44:47 openstackgerrit Adam Spiers proposed openstack/nova master: Move get_machine_type() test to test_utils.py https://review.opendev.org/680527
21:57:03 efried_afk dansmith: vpmem json blob => hyp-specific ovo (https://review.opendev.org/#/c/678448/), you said to follow the example of LiveMigrateData.
21:57:03 efried_afk So I think that means they should make a base class (e.g. ResourceMetadata) and make LibvirtPMEMDevice (https://review.opendev.org/#/c/678453/19/nova/objects/resource.py) a subclass thereof.
21:57:03 efried_afk I can't see where LiveMigrateData is a field in another ovo, though; is it kosher to make Resource.metadata of type ResourceMetadata even though IRL it will always be a *subclass*?
21:57:39 openstackgerrit Matt Riedemann proposed openstack/nova master: Rename Claims resources to compute_node https://review.opendev.org/679470
21:58:13 dansmith yeah, although you might have to register the base to make that work, which I don't think was required for the MD case because we always sent the child and not the parent, IIRC, but it's been a whiel
21:58:33 efried_afk what does "register the base" mean?
21:59:40 mriedem @obj_base.NovaObjectRegistry.register
22:00:10 mriedem note that the base LiveMigrateData is not registered
22:00:11 mriedem https://github.com/openstack/nova/blob/master/nova/objects/migrate_data.py#L112

Earlier   Later