| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-09-05 | |||
| 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 | |
| 22:00:16 | mriedem | b/c everything uses the subclasses | |
| 22:00:25 | dansmith | right that ^ | |
| 22:00:48 | dansmith | efried: ovo requires a shared registry of the schemas on either side to ensure proper versioning | |
| 22:00:50 | efried | everything will use the subclass in this case too... unless you mean that the Resource object will "use" the base class? | |
| 22:00:55 | mriedem | so i can't do something like this in my driver: migrate_data = objects.LiveMigrateData(all_my_cool_shenanigans={...}) | |
| 22:01:07 | dansmith | efried: the difference is the inclusion in a field, which has to specify an object in the registry, IIRC | |
| 22:01:14 | dansmith | which is why it's by name and not by reference | |
| 22:01:17 | efried | okay, cool. | |
| 22:03:58 | efried | dansmith, mriedem: Did I say it right? https://review.opendev.org/#/c/678448/15/nova/objects/resource.py@36 | |
| 22:05:01 | mriedem | i don't know that the chinese are going to understand the south park underpants gnomes reference | |
| 22:05:20 | efried | I've never actually even seen that thing | |
| 22:05:33 | efried | indirect meme reference culture | |
| 22:05:52 | dansmith | efried: commented | |
| 22:05:55 | efried | I could find it if I wanted | |
| 22:06:33 | efried | dansmith: thanks. I think nullable because not all ResourceZ are going to have metadata. | |
| 22:06:41 | mriedem | then don't set the field | |
| 22:06:44 | dansmith | efried: yeah, that's the wrong answer | |
| 22:06:47 | efried | ah | |
| 22:07:25 | efried | unset vs value of None vs object with no fields initialized vs object with fields set to None vs ..... | |
| 22:07:26 | dansmith | I thought I already commented about this in this patch, but maybe I was overwhelmed by the larger sin of using a string here and it didn't come through | |
| 22:07:42 | mriedem | i need to make dan a shirt that has nullable=True on some object field with the circle bar banned icon on it | |
| 22:07:49 | dansmith | I'd wear it | |
| 22:08:17 | dansmith | efried: I suspect the right thing there is to always have a metadata object in that field (hence never None), with either fields set appropriately empty or whatever else, | |
| 22:08:26 | dansmith | but it's hard to tell what the right thing is because it's not defined there | |